feat: Added Nullability markup for some classes - #1710
feat: Added Nullability markup for some classes#1710Maggie Kimani (MaggieKimani1) merged 11 commits into
Conversation
|
We reviewed these changes and I think we understand the incremental step you are trying to take. This is consistent with the direction that we want to go for the v2 release. Thank you for making these changes. Be aware that there are likely going to still be a fair amount of code churn in this branch, so the kind of changes that still need to be made may cause merge conflicts. |
|
It seems we need to update System.Text.Json to 8.0.4 because It doesn't allow to pass CICD. After the update, some of the tests fail, so this needs to be done in a separate PR with update/fix for tests |
|
Konstantin S. (@HavenDV) good call. And make sure we're using version ranges for library projects. Thanks a lot! |
Actually this has already been handled in #1729 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Yes, I saw this so I decided not to risk making parallel changes here. |
|
Vincent Biret (@baywet) I'm not sure, but maybe it's blocked by "baywet requested changes" status. Is there anything else that needs fixing? |
Vincent Biret (baywet)
left a comment
Vincent Biret (baywet)
left a comment
There was a problem hiding this comment.
Sorry, I forgot to reset my review
|
Maggie Kimani (@MaggieKimani1) Could we get this PR merged before we get further conflicts? |
# Conflicts: # src/Microsoft.OpenApi/Models/OpenApiComponents.cs # src/Microsoft.OpenApi/Models/OpenApiDocument.cs # src/Microsoft.OpenApi/Models/OpenApiMediaType.cs # test/Microsoft.OpenApi.Hidi.Tests/Formatters/PowerShellFormatterTests.cs # test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt
…s according this.
5 participants
Related to #1202
I think before that, #1202 should be split into two parts - marking the current code as is without changing the behavior
And after that, for the next big update, REQUIRED things should be replaced so that they are not null even in an empty document
This PR does not change behavior in places where null guards were provided or null was expected.
In places where there were NREs, ? were added. or other trivial fixes to avoid NRE
I'll continue with nullable markup if the change is accepted quickly enough